Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RegressionTest error message for when possible non-deterministic behavior is detected. #883

Merged

Conversation

jdcove2
Copy link
Collaborator

@jdcove2 jdcove2 commented Aug 8, 2024

This PR adds an error message when RegressionTest is generating answers and differences are detected. This usually indicates that there is non-deterministic state in the IBDO because the place is executed once to generate the answer file and once to check against the answer file. If these two executions produce different results, then this most likely indicates non-deterministic state.

@cfkoehler cfkoehler self-requested a review August 12, 2024 15:22
@cfkoehler
Copy link
Collaborator

Do you have an issue or ticket that initiated this work?
I think this is something we should add just curious on the motivation for doing it now.

@jdcove2
Copy link
Collaborator Author

jdcove2 commented Aug 12, 2024

Do you have an issue or ticket that initiated this work? I think this is something we should add just curious on the motivation for doing it now.

Yes. I was working with @fbruton, who was using RegressionTest, and we could not initially figure out why RegressionTest was complaining about "differences" even though he was having RegressionTest generate the answer XML file. I looked into it and figured out that the place was producing "non-deterministic state". This means that each execution of the place with the same input IBDO produced different output IBDO's. The error message given by RegressionTest did not give any indication that this was the situation. This PR updates the error message so that the user of RegressionTest is given additional information on what is causing RegressionTest to show that there are "differences".

Copy link
Collaborator

@drivenflywheel drivenflywheel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor verbiage change requested, but otherwise looks good

@cfkoehler cfkoehler added this to the v8.9.0 milestone Aug 13, 2024
@cfkoehler cfkoehler merged commit 6b49867 into NationalSecurityAgency:main Aug 14, 2024
13 checks passed
@cfkoehler cfkoehler added the test-only The change only impacts test code label Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test-only The change only impacts test code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants